Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: Add more accessors for Metadata on Windows #62980

Merged
merged 1 commit into from
Jul 26, 2019

Conversation

alexcrichton
Copy link
Member

This commit adds accessors for more fields in fs::Metadata on Windows
which weren't previously exposed. There's two sources of fs::Metadata
on Windows currently, one from DirEntry and one from a file itself.
These two sources of information don't actually have the same set of
fields exposed in their stat information, however. To handle this the
platform-specific accessors of Windows-specific information all return
Option to return None in the case a metadata comes from a
DirEntry, but they're guaranteed to return Some if it comes from a
file itself.

This is motivated by some changes in CraneStation/wasi-common#42, and
I'm curious how others feel about this platform-specific functionality!

@rust-highfive
Copy link
Collaborator

r? @bluss

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2019
@alexcrichton
Copy link
Member Author

r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned bluss Jul 25, 2019
Copy link
Contributor

@marmistrz marmistrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@sfackler
Copy link
Member

r=me with a tracking issue filled out

@bors
Copy link
Contributor

bors commented Jul 26, 2019

☔ The latest upstream changes (presumably #60260) made this pull request unmergeable. Please resolve the merge conflicts.

@marmistrz
Copy link
Contributor

@alexcrichton I thought about the change once more: why do make a refactor and radically change the definition of FileAttr instead of just storing an Option<u32> or Option<u64> in FileAttr (by adding new fields)?

This solution has the upside of not having to pattern match during runtime - you could just put the Nones manually for DirEntry and explicit Somes for fs::metadata

This commit adds accessors for more fields in `fs::Metadata` on Windows
which weren't previously exposed. There's two sources of `fs::Metadata`
on Windows currently, one from `DirEntry` and one from a file itself.
These two sources of information don't actually have the same set of
fields exposed in their stat information, however. To handle this the
platform-specific accessors of Windows-specific information all return
`Option` to return `None` in the case a metadata comes from a
`DirEntry`, but they're guaranteed to return `Some` if it comes from a
file itself.

This is motivated by some changes in CraneStation/wasi-common#42, and
I'm curious how others feel about this platform-specific functionality!
@alexcrichton
Copy link
Member Author

@marmistrz I generally personally prefer to keep the original structures around for as long as possible, but the recent addition of UWP made that untenable anyway, so I went ahead and did what you're thinking anyway

@bors: r=sfackler

@bors
Copy link
Contributor

bors commented Jul 26, 2019

📌 Commit c69f367 has been approved by sfackler

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 26, 2019
…ackler

std: Add more accessors for `Metadata` on Windows

This commit adds accessors for more fields in `fs::Metadata` on Windows
which weren't previously exposed. There's two sources of `fs::Metadata`
on Windows currently, one from `DirEntry` and one from a file itself.
These two sources of information don't actually have the same set of
fields exposed in their stat information, however. To handle this the
platform-specific accessors of Windows-specific information all return
`Option` to return `None` in the case a metadata comes from a
`DirEntry`, but they're guaranteed to return `Some` if it comes from a
file itself.

This is motivated by some changes in CraneStation/wasi-common#42, and
I'm curious how others feel about this platform-specific functionality!
bors added a commit that referenced this pull request Jul 26, 2019
Rollup of 22 pull requests

Successful merges:

 - #62084 (allow clippy::unreadable_literal in unicode tables)
 - #62120 (Add missing type links in documentation)
 - #62310 (Add missing doc links in boxed module)
 - #62421 (Introduce `as_deref` to Option)
 - #62583 (Implement Unpin for all raw pointers)
 - #62692 (rustc: precompute the largest Niche and store it in LayoutDetails.)
 - #62801 (Remove support for -Zlower-128bit-ops)
 - #62828 (Remove vector fadd/fmul reduction workarounds)
 - #62862 (code cleanup)
 - #62904 (Disable d32 on armv6 hf targets)
 - #62907 (Initialize the MSP430 AsmParser)
 - #62956 (Implement slow-path for FirstSets::first)
 - #62963 (Allow lexer to recover from some homoglyphs)
 - #62964 (clarify and unify some type test names)
 - #62970 (ci: gate toolstate repo pushes on the TOOLSTATE_PUBLISH envvar)
 - #62980 (std: Add more accessors for `Metadata` on Windows)
 - #62983 (Remove needless indirection through Rc)
 - #62985 (librustc_errors: Support ui-testing flag in annotate-snippet emitter)
 - #63002 (error_index_generator should output stdout/stderr when it panics.)
 - #63004 (Add test for issue-54062)
 - #63007 (ci: debug network failures while downloading awscli from PyPI)
 - #63009 (Remove redundant `mut` from variable declaration.)

Failed merges:

r? @ghost
@bors bors merged commit c69f367 into rust-lang:master Jul 26, 2019
@alexcrichton alexcrichton deleted the windows-metadata branch July 31, 2019 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants